<!---{
"title": "Creating a new diagnostic in quick-lint-js",
"navTitle": "Diagnostics"
}--->

<!DOCTYPE html>
<!-- Copyright (C) 2020  Matthew "strager" Glazar -->
<!-- See end of file for extended copyright information. -->
<html>
  <head>
    <%- await include("../../common-head.ejs.html") %>
    <link href="../../main.css" rel="stylesheet" />
    <style>
      pre mark {
        text-decoration: none;
      }

      mark.unimportant {
        opacity: 0.5;
      }

      .arg-1 {
        --arg-color: #ffb;
      }
      .arg-2 {
        --arg-color: #fbf;
      }
      .arg-3 {
        --arg-color: #aff;
      }
      .arg-4 {
        --arg-color: #fcc;
      }
      .arg-5 {
        --arg-color: #cfc;
      }
      .arg-6 {
        --arg-color: #ccf;
      }

      @media (prefers-color-scheme: dark) {
        .arg-1 {
          --arg-color: #552;
        }
        .arg-2 {
          --arg-color: #525;
        }
        .arg-3 {
          --arg-color: #255;
        }
        .arg-4 {
          --arg-color: #522;
        }
        .arg-5 {
          --arg-color: #252;
        }
        .arg-6 {
          --arg-color: #236;
        }
      }

      mark.arg-1,
      mark.arg-2,
      mark.arg-3,
      mark.arg-4,
      mark.arg-5,
      mark.arg-6,
      .mark-reference {
        background-color: var(--arg-color);
      }

      figure {
        margin-left: 2rem;
        margin-right: 2rem;
      }

      .gdb-session kbd {
        font-weight: bold;
      }
      .gdb-session samp {
        opacity: 0.5;
      }
      .gdb-session samp.important {
        opacity: 1;
      }
    </style>
  </head>
  <body class="side-bar-nav">
    <header><%- await include("../../common-nav.ejs.html") %></header>

    <main>
      <h1><%= meta.title %></h1>

      <p>
        A common task when contributing to quick-lint-js is to create a new
        diagnostic. In quick-lint-js, <dfn>diagnostic</dfn> is a warning or
        error reported while parsing the user's JavaScript code.
      </p>

      <p>Creating a diagnostic involves four pieces of code:</p>
      <ol>
        <li><a href="#diagnostic-type">Diagnostic type and metadata</a></li>
        <li><a href="#test">Test for the diagnostic</a></li>
        <li><a href="#report">Detection and reporting of the diagnostic</a></li>
        <li><a href="#document">Documentation for the website</a></li>
      </ol>

      <section id="diagnostic-type">
        <h2>1. Diagnostic type and metadata</h2>

        <p>
          Diagnostic types are listed in
          <a
            href="https://github.com/quick-lint/quick-lint-js/blob/master/src/quick-lint-js/diag/diagnostic-types-2.h"
            ><code>src/quick-lint-js/diag/diagnostic-types-2.h</code></a
          >. They look like this:
        </p>
        <figure>
          <pre><code class="cxx">struct <mark class="arg-1">Diag_Missing_Body_For_Try_Statement</mark> {
  [[qljs::diag(<mark class="arg-2">"E0120"</mark>, <mark class="arg-3">Diagnostic_Severity::error</mark>)]] //
  [[qljs::message(<mark class="arg-5">"missing body for try statement",</mark>
                  <mark class="arg-5">ARG(try_token)</mark>)]]  //
  <mark class="arg-4">Source_Code_Span try_token;</mark>
};</code></pre>
        </figure>

        <p>
          Each diagnostic type is a C++
          <code class="cxx">struct</code> with some custom C++ [[attributes]].
        </p>

        <p>The C++ compiler ignores the attributes and just sees a class:</p>

        <figure>
          <pre><code class="cxx">struct <mark class="arg-1">Diag_Missing_Body_For_Try_Statement</mark> {
    <mark class="arg-4">Source_Code_Span try_token;</mark>
};</code></pre>
        </figure>

        <p>
          The custom attributes are processed by a separate tool as part of
          quick-lint-js's build system. We will discuss the custom attributes
          shortly.
        </p>

        <p>
          Let's pick a <span class="arg-1 mark-reference">name</span> for our
          diagnostic. The name needs to start with
          <code class="cxx">Diag_</code> and be a legal C++ class name. We will
          use this name later to report the diagnostic and to test for the
          diagnostic:
        </p>
        <figure>
          <pre><code class="cxx">struct <mark class="arg-1">Diag_Comparison_With_Empty_String</mark> {
  [[qljs::diag(<mark class="arg-2 unimportant">/* TODO */</mark>, <mark class="arg-3 unimportant">/* TODO */</mark>)]]  //
  [[qljs::message(<mark class="arg-5 unimportant">/* TODO */</mark>)]]           //
  <mark class="arg-4 unimportant">/* TODO */</mark>
};</code></pre>
        </figure>

        <p>
          Each diagnostic in quick-lint-js has a unique diagnostic code as the
          <span class="arg-2 mark-reference">first argument</span> to
          <code class="cxx">[[qljs::diag]]</code>. A diagnostic code is the
          letter <code>E</code> followed by four decimal digits. Let's be lazy
          and reuse an existing diagnostic code for now:
        </p>
        <figure>
          <pre><code class="cxx">struct <mark class="arg-1 unimportant">Diag_Comparison_With_Empty_String</mark> {
  [[qljs::diag(<mark class="arg-2">"E0001"</mark>, <mark class="arg-3 unimportant">/* TODO */</mark>)]]  //
  [[qljs::message(<mark class="arg-5 unimportant">/* TODO */</mark>)]]        //
  <mark class="arg-4 unimportant">/* TODO */</mark>
};</code></pre>
        </figure>

        <p>
          We also need to pick a diagnostic severity, either
          <code class="cxx">Diagnostic_Severity::error</code> or
          <code class="cxx">Diagnostic_Severity::warning</code>, as the
          <span class="arg-3 mark-reference">second argument</span> to
          <code class="cxx">[[qljs::diag]]</code>. Our new diagnostic is for a
          <em>possible</em> issue, so let's pick
          <code class="cxx">warning</code>:
        </p>
        <figure>
          <pre><code class="cxx">struct <mark class="arg-1 unimportant">Diag_Comparison_With_Empty_String</mark> {
  [[qljs::diag(<mark class="arg-2 unimportant">"E0001"</mark>, <mark class="arg-3">Diagnostic_Severity::warning</mark>)]]  //
  [[qljs::message(<mark class="arg-5 unimportant">/* TODO */</mark>)]]                          //
  <mark class="arg-4 unimportant">/* TODO */</mark>
};</code></pre>
        </figure>

        <p>
          Each diagnostic is a class (<code class="cxx">struct</code>). The
          class needs to store at least one
          <code class="cxx">Source_Code_Span</code> member variable so the
          editor knows where to put the
          <span class="diagnostic">squigglies</span>. We should think about
          where the squigglies should go and name our member variable
          appropriately to make the reporting and testing code easier to read.
          Since our diagnostic is about string comparisons, let's name the
          member variable <code class="cxx">comparison_operator</code>. We write
          the <span class="arg-4 mark-reference">member variables</span> after
          the C++ attributes:
        </p>
        <figure>
          <pre><code class="cxx">struct <mark class="arg-1 unimportant">Diag_Comparison_With_Empty_String</mark> {
  [[qljs::diag(<mark class="arg-2 unimportant">"E0001"</mark>, <mark class="arg-3 unimportant">Diagnostic_Severity::warning</mark>)]]  //
  [[qljs::message(<mark class="arg-5 unimportant">/* TODO */</mark>)]]                          //
  <mark class="arg-4">Source_Code_Span comparison_operator;</mark>
};</code></pre>
        </figure>

        <p>
          Each diagnostic needs a message specified by one or more
          <code class="arg-5 mark-reference">[[qljs::message]]</code>
          attributes. Most diagnostics are simple and just have a simple string.
          Other diagnostics might have formatting or multiple strings. Our
          diagnostic is simple, so let's just write a single string with no
          formatting. Don't forget to mention the
          <code class="cxx">Source_Code_Span</code> member variable we defined
          inside our <code class="cxx">Diag_</code> class:
        </p>
        <figure>
          <pre><code class="cxx">struct <mark class="arg-1 unimportant">Diag_Comparison_With_Empty_String</mark> {
  [[qljs::diag(<mark class="arg-2 unimportant">"E0001"</mark>, <mark class="arg-3 unimportant">Diagnostic_Severity::warning</mark>)]]  //
  [[qljs::message(<mark class="arg-5">"comparing against empty strings is silly",</mark>
                  <mark class="arg-5">ARG(comparison_operator)</mark>)]]  //
  <mark class="arg-4 unimportant">Source_Code_Span comparison_operator;</mark>
};</code></pre>
        </figure>

        <p>
          After adding the diagnostic type to <code>diagnostic-types-2.h</code>,
          build quick-lint-js. You should get a build error like the following:
        </p>
        <figure class="build-error">
          <pre style="white-space: pre-wrap">
diagnostic-types-2.h:655:20: error: diag code "E0001" already in use; try this unused diag code: "E0331"
diagnostic-types-2.h:107:16: note: Diag_Assignment_Before_Variable_Declaration used code "E0001" here</pre
          >
          <figcaption>Build error after creating a diagnostic type</figcaption>
        </figure>

        <p>
          A build check is telling us that the error code we chose
          (<code>E0001</code>) is already in use. Let's change our
          <code class="cxx">Diag_</code> class in
          <code>diagnostic-types-2.h</code> to use the unused diagnostic code
          suggested by the check:
        </p>
        <figure>
          <pre><code class="cxx">struct <mark class="arg-1 unimportant">Diag_Comparison_With_Empty_String</mark> {
  [[qljs::diag(<mark class="arg-2">"E0331"</mark>, <mark class="arg-3 unimportant">Diagnostic_Severity::warning</mark>)]]  //
  [[qljs::message(<mark class="arg-5 unimportant">"comparing against empty strings is silly",</mark>
                  <mark class="arg-5 unimportant">ARG(comparison_operator)</mark>)]]  //
  <mark class="arg-4 unimportant">Source_Code_Span comparison_operator;</mark>
};</code></pre>
        </figure>

        <p>
          Now let's build quick-lint-js again and run the tests. We should get
          no failures, which means we didn't break anything:
        </p>
        <figure>
          <pre><code class="shell-session"><kbd>ninja -C build quick-lint-js-test</kbd>
ninja: Entering directory `build'
[103/103] Linking CXX executable test/quick-lint-js-test

<kbd>./build/test/quick-lint-js-test --gtest_brief=1</kbd>
Running main() from gmock_main.cc
[==========] 1980 tests from 151 test suites ran. (772 ms total)
[  PASSED  ] 1980 tests.</code></pre>
        </figure>

        <p>
          The build scripts modified a few files for us. Make sure you include
          these files in your commit:
        </p>
        <figure>
          <pre><code class="shell-session"><kbd>git status</kbd>
git status
On branch dev3
Changes not staged for commit:
  (use "git add &lt;file&gt;..." to update what will be committed)
  (use "git restore &lt;file&gt;..." to discard changes in working directory)
        modified:   po/messages.pot
        modified:   src/quick-lint-js/diag/diagnostic-metadata-generated.cpp
        modified:   src/quick-lint-js/diag/diagnostic-types-2.h
        modified:   src/quick-lint-js/i18n/translation-table-generated.cpp
        modified:   src/quick-lint-js/i18n/translation-table-generated.h
        modified:   src/quick-lint-js/i18n/translation-table-test-generated.h

no changes added to commit (use "git add" and/or "git commit -a")</code></pre>
        </figure>

        <p>
          Now that we have created the diagnostic type, let's move on to writing
          a test.
        </p>
      </section>

      <section id="test">
        <h2>2. Test for the diagnostic</h2>

        <p>
          All diagnostics must be tested with an automated test. To create a
          test, copy-paste an existing test in a
          <code>test/test-parse-*.cpp</code> file and tweak it. Let's put our
          test in
          <a
            href="https://github.com/quick-lint/quick-lint-js/blob/master/test/test-parse-warning.cpp"
            ><code>test/test-parse-warning.cpp</code></a
          >:
        </p>

        <figure>
          <pre><code class="cxx">TEST_F(Test_Parse_Warning, warn_on_empty_string_literal_comparison) {
  <mark class="arg-3">test_parse_and_visit_expression</mark>(
      <mark class="arg-1">u8"a === ''"_sv</mark>,  //
      u8"<mark class="arg-6">  ^^^</mark> <mark class="arg-4">Diag_Comparison_With_Empty_String</mark>"_diag);
}</code></pre>
        </figure>

        <p>There are a few pieces of this test worth mentioning:</p>
        <dl>
          <dt>
            <code class="cxx"
              ><span class="mark-reference arg-3"
                >test_parse_and_visit_expression</span
              ></code
            >
          </dt>
          <dd>
            quick-lint-js' parser can parse several things, including
            statements, expressions, and TypeScript types. Our diagnostic is
            specific to JavaScript expressions, so we call
            <code class="cxx">test_parse_and_visit_expression</code>.
          </dd>

          <dt>
            <code class="cxx"
              ><span class="mark-reference arg-1">u8"a == ''"_sv</span></code
            >
          </dt>
          <dd>
            The input source code we want to test. The
            <code class="cxx">u8</code> prefix is required so the code is parsed
            as UTF-8. The <code class="cxx">_sv</code> suffix is required so
            that code containing null bytes is handled correctly.
          </dd>

          <dt>
            <code class="cxx"
              ><span class="mark-reference arg-6">&nbsp;&nbsp;^^^</span></code
            >
          </dt>
          <dd>
            We need to tell
            <code class="cxx">test_parse_and_visit_expression</code> where in
            the source code the diagnostic should be reported. This is
            represented using two parts inside a string: alignment (spaces) and
            a span (one or more <code>^</code>s, or one <code>`</code>). In our
            example, there are two leading spaces, so the diagnostic should
            start at the third character (byte offset 2). The span is three
            characters wide (<code>^^^</code>), so the diagnostic covers three
            characters (up until byte offset 5). The alignment and span specify
            that the diagnostic should cover offsets 2, 3, and 4:
            <figure>
              <pre><code>a === ''
<span class="mark-reference arg-6">  ^^^</span>
  offsets 2, 3, and 4</code></pre>
            </figure>
          </dd>

          <dt>
            <code class="cxx"
              ><span class="mark-reference arg-4"
                >Diag_Comparison_With_Empty_String</span
              ></code
            >
          </dt>
          <dd>
            We need to tell
            <code class="cxx">test_parse_and_visit_expression</code> which kind
            of diagnostic we expect. We do this by writing the diagnostic
            class's name after the span.

            <p>
              If a diagnostic has multiple members (such as if the diagnostic
              has multiple messages), the member name must appear after the
              diagnostic class's name. See the
              <a
                href="https://github.com/quick-lint/quick-lint-js/blob/master/test/quick-lint-js/diagnostic-assertion.h#L20"
                >documentation for <code class="cxx">_diag</code> and
                NOTE[_diag-syntax]</a
              >
              for details.
            </p>
          </dd>
        </dl>

        <p>
          Build and run the test to make sure it fails. The failure says that we
          expected a diagnostic, but didn't get any. This makes sense because we
          haven't written the code to report the diagnostic yet:
        </p>
        <figure>
          <pre><code class="shell-session"><kbd>ninja -C build quick-lint-js-test</kbd>
ninja: Entering directory `build'
[2/2] Linking CXX executable test/quick-lint-js-test

<kbd>./build/test/quick-lint-js-test --gtest_brief=1</kbd>
Running main() from gmock_main.cc
/home/strager/Projects/quick-lint-js-sl/test/test-parse-warning.cpp:561: Failure
Value of: p.errors
<mark>Expected: has 1 element that has type Diag_Comparison_With_Empty_String</mark>
  <mark>Actual: {}</mark>
[  FAILED  ] Test_Parse_Warning.warn_on_empty_string_literal_comparison (0 ms)
main thread ID: 373982
[==========] 1981 tests from 151 test suites ran. (755 ms total)
[  PASSED  ] 1980 tests.</code></pre>
        </figure>
      </section>

      <section id="report">
        <h2>3. Detection and reporting of the diagnostic</h2>

        <p>
          Now for the hard part: writing the production code. Most likely we
          will report our diagnostic in one of these files:
        </p>
        <ul>
          <li>
            <a
              href="https://github.com/quick-lint/quick-lint-js/blob/master/src/quick-lint-js/fe/lex.cpp"
              >src/quick-lint-js/fe/lex.cpp</a
            >
          </li>
          <li>
            <a
              href="https://github.com/quick-lint/quick-lint-js/blob/master/src/quick-lint-js/fe/parse.cpp"
              >src/quick-lint-js/fe/parse.cpp</a
            >
            or parse-*.cpp
          </li>
          <li>
            <a
              href="https://github.com/quick-lint/quick-lint-js/blob/master/src/quick-lint-js/fe/variable-analyzer.cpp"
              >src/quick-lint-js/fe/variable-analyzer.cpp</a
            >
          </li>
        </ul>

        <p>
          But these files contain thousands of lines of code. How do we know
          where to put our new code?
        </p>

        <p>One technique is to step through the code in a debugger:</p>

        <figure>
          <pre
            class="gdb-session"
          >$ <kbd>gdb --args ./build/test/quick-lint-js-test --gtest_filter=Test_Parse_Warning.warn_on_empty_string_literal_comparison</kbd>
(gdb) <kbd>b parse_and_visit_expression</kbd>
<samp class="important">Breakpoint 1 at 0x754ae0: parse_and_visit_expression. (3 locations)</samp>
(gdb) <kbd>run</kbd>
<samp>Starting program: build/test/quick-lint-js-test --gtest_filter=Test_Parse_Warning.warn_on_empty_string_literal_comparison
Running main() from gmock_main.cc
Note: Google Test filter = Test_Parse_Warning.warn_on_empty_string_literal_comparison
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from Test_Parse_Warning
[ RUN      ] Test_Parse_Warning.warn_on_empty_string_literal_comparison

Breakpoint 1, quick_lint_js::Test_Parser::parse_and_visit_expression at test/./quick-lint-js/parse-support.h:97
97          this-&gt;parser_.parse_and_visit_expression(this-&gt;errors_);</samp>
(gdb) <kbd>c</kbd>
<samp>Continuing.

Breakpoint 1, quick_lint_js::Parser::parse_and_visit_expression at src/./quick-lint-js/fe/parse.h:174
174         this-&gt;parse_and_visit_expression(v, precedence{});</samp>
(gdb) <kbd>c</kbd>
<samp>Continuing.

Breakpoint 1, quick_lint_js::Parser::parse_and_visit_expression at src/./quick-lint-js/fe/parse.h:550
550         Monotonic_Allocator &amp;alloc = *this-&gt;expressions_.allocator();</samp>
(gdb) <kbd>n</kbd>
<samp>551         auto rewind_guard = alloc.make_rewind_guard();</samp>
(gdb) <kbd>n</kbd>
<samp>553         Expression *ast = this-&gt;parse_expression(v, prec);</samp>
(gdb) <kbd>n</kbd>
<samp>555           auto disable_guard = alloc.disable();</samp>
(gdb) <kbd>n</kbd>
<samp class="important">556           this-&gt;visit_expression(ast, v, variable_context::rhs);</samp>
(gdb) <kbd>step</kbd>
<samp>quick_lint_js::Parser::visit_expression at src/quick-lint-js/fe/parse-expression.cpp:36
36        auto visit_children = [&amp;] {</samp>
(gdb) <kbd>n</kbd>
<samp>41        switch (ast-&gt;kind()) {</samp>
(gdb) <kbd>n</kbd>
<samp>67          visit_children();</samp>
(gdb) <kbd>n</kbd>
<samp>69          static_cast&lt;Expression::Binary_Operator*&gt;(ast));</samp>
(gdb) <kbd>n</kbd>
<samp class="important">68          this-&gt;error_on_pointless_compare_against_literal(</samp></pre>
        </figure>

        <p>
          <code class="cxx">error_on_pointless_compare_against_literal</code>
          looks like a good place to put our code.
        </p>

        <p>
          Detecting when to report the diagnostic is up to you. But once you
          have the information you need, reporting a diagnostics is easy:
        </p>

        <figure>
          <pre><code class="cxx">Source_Code_Span op_span = /* usually this->peek().span */;
this->diags_.add(Diag_Comparison_With_Empty_String{
    .comparison_operator = op_span,
});</code></pre>
        </figure>

        <p>Build and test to prove that our code worked:</p>
        <figure>
          <pre><code class="shell-session"><kbd>ninja -C build quick-lint-js-test</kbd>
ninja: Entering directory `build'
[3/3] Linking CXX executable test/quick-lint-js-test

<kbd>./build/test/quick-lint-js-test --gtest_brief=1</kbd>
Running main() from gmock_main.cc
main thread ID: 375845
[==========] 1981 tests from 151 test suites ran. (785 ms total)
[  PASSED  ] 1981 tests.</code></pre>
        </figure>

        <p>Huzzah! 🥳</p>

        <p>
          But we're not done yet... We still have to write 💀
          <a href="#document">documentation</a> 💀
        </p>
      </section>

      <section id="document">
        <h2>4. Documentation for the website</h2>

        <p>
          Each diagnostic has associated documentation stored separately from
          the code. The docs are stored in
          <a
            href="https://github.com/quick-lint/quick-lint-js/tree/master/docs/errors"
            >docs/errors/</a
          >
          with one file per diagnostic. Let's write our documentation:
        </p>

        <figure>
          <pre><code class="markdown"># <mark class="arg-1">E0331: comparing against empty strings is silly</mark>

Empty strings are error-prone, and comparing against empty strings is extra
error-prone:

<mark class="arg-2">```javascript
let x = prompt();
if (x === '') {
    alert('You were supposed to type something!');
}
```</mark>

To fix this mistake, check the string's `length` property instead:

<mark class="arg-3">```javascript
let x = prompt();
if (x.length === 0) {
    alert('You were supposed to type something!');
}
```</mark>

Alternatively, treat the string as a boolean:

<mark class="arg-4">```javascript
let x = prompt();
if (!x) {
    alert('You were supposed to type something!');
}
```</mark>

(This is an example diagnostic for contributor documentation purposes. Comparing
against empty strings is totally fine, and the fact that quick-lint-js reports a
diagnostic in this case is sad.)</code></pre>
        </figure>

        <p>Some important parts of diagnostic documentation:</p>
        <dl>
          <dt><span class="arg-1 mark-reference">title</span></dt>
          <dd>
            The title of the document should include the diagnostic's code. The
            diagnostic's message should follow the code. The title should
            include the message mentioned in <code>diagnostic-types-2.h</code>,
            but it doesn't have to match exactly. Interpolation markers such as
            <code>{1:headlinese}</code> should be omitted.
          </dd>

          <dt><span class="arg-2 mark-reference">first example</span></dt>
          <dd>
            The first code snippet should be fenced with
            <code class="markdown">```javascript</code> or
            <code class="markdown">```typescript</code> (or another other
            support language). This code snippet demonstrates broken code and
            <strong>must</strong> cause quick-lint-js to report a diagnostic. A
            broken code snippet is <strong>required</strong>.
          </dd>

          <dt><span class="arg-3 mark-reference">second example</span></dt>
          <dd>
            The second code snippet should also be fenced. This code snippet
            demonstrates working code and <strong>must not</strong> cause
            quick-lint-js to report any diagnostic. A working code snippet is
            <strong>required</strong>.
          </dd>

          <dt><span class="arg-4 mark-reference">extra examples</span></dt>
          <dd>
            You can include more code snippets after the second. Each of these
            extra code snippet must cause no diagnostics. Usually these code
            snippets show alternate ways of addressing the original issue. These
            extra examples are optional.
          </dd>
        </dl>
      </section>
    </main>

    <footer><%- await include("../../common-footer-nav.ejs.html") %></footer>
  </body>
</html>

<!--
quick-lint-js finds bugs in JavaScript programs.
Copyright (C) 2020  Matthew "strager" Glazar

This file is part of quick-lint-js.

quick-lint-js is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.

quick-lint-js is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with quick-lint-js.  If not, see <https://www.gnu.org/licenses/>.
-->
